Invert canvas rotation direction if view is flipped #1778
Conversation
There was a problem hiding this comment.
Hi Henrique
Thanks again for contributing 😄
I have a few things this time around, although the code itself is fine, I think we can do a better than simply copy/pasting, which could introduce bugs down the line.
There's also the usage of bitwise XOR, which is not something we use in the project afaik. I don't personally have a stance on that as I think there's something to be said about its simplicity but it could introduce bugs... as such I think it may be better to use conventional logical operators instead, like:
if (A && !B || B && !A)
Note: Don't feel disheartened by the lack of approval right away, there's nothing wrong with the code, it just needs a bit of tweaking. We strive for simple and readable while also trying to minimize the possibility of bugs.
similar to `rotate()` but doesn't require caller to know the current view rotation.
as requested by MrStevns.
- replace XOR operators with `A == !B` - replace `if` with ternary operator - use rotateRelative() instead of rotate() - define a delta constant and call rotateRelative() once
- replace `if` with ternary operator - replace rotate() with rotateRelative()
|
Apologies for the delay in committing the last changes, I've made some improvements and re-tested everything. :) Please review once again! |
Fixes #1777.
Please note that I am not an experienced C++ developer, so please let me know if something can be improved!
This PR adds a check to
ActionCommands::rotateClockwise(),ActionCommands::rotateCounterClockwise()andHandTool::transformView()that inverts the rotation direction if the view is flipped either horizontally or vertically (but not both), so it's more intuitive:Shortcuts (Z then R):
https://github.com/pencil2d/pencil/assets/75134774/ac722475-312d-4ac9-aeee-42c81653dcd6
Hand Tool:
https://github.com/pencil2d/pencil/assets/75134774/5e6d216b-31dd-4e88-844d-06c87b3a86c9